Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1054 +/- ##
==========================================
- Coverage 97.68% 97.20% -0.48%
==========================================
Files 160 161 +1
Lines 6600 6648 +48
==========================================
+ Hits 6447 6462 +15
- Misses 153 186 +33 ☔ View full report in Codecov by Sentry. |
DominicOram
left a comment
DominicOram
left a comment
There was a problem hiding this comment.
Thanks! Couple of comments, I think there are some additional things we need to set too:
num_frames_chunks- The file name/directory e.g. everything in
set_odin_pvs dev_shm
| def load_metadata( | ||
| eiger: EigerDetector, energy: float, enable: bool, detector_params: DetectorParams | ||
| ): | ||
| def _inner_plan(): |
There was a problem hiding this comment.
Should: Why do you need an inner plan here?
There was a problem hiding this comment.
I think it was the first thing I wrote for structure and left in for no reason; removing as definitely not needed :0
| def set_mx_settings_pvs(eiger: EigerDetector, detector_params: DetectorParams): | ||
| beam_x_pixels, beam_y_pixels = detector_params.get_beam_position_pixels( | ||
| detector_params.detector_distance | ||
| ) | ||
|
|
||
| yield from bps.abs_set(eiger.drv.beam_centre_x, beam_x_pixels) | ||
| yield from bps.abs_set(eiger.drv.beam_centre_y, beam_y_pixels) | ||
| yield from bps.abs_set(eiger.drv.det_distance, detector_params.detector_distance) | ||
|
|
||
| yield from bps.abs_set(eiger.drv.omega_start, detector_params.omega_start) | ||
| yield from bps.abs_set(eiger.drv.omega_increment, detector_params.omega_increment) |
There was a problem hiding this comment.
Should: We probably want to group these things as it makes it more async (even if Tom/Gary assure us it's instantaneous). Then we will need to wait on them at some point so we should add that optionally too e.g.
| def set_mx_settings_pvs(eiger: EigerDetector, detector_params: DetectorParams): | |
| beam_x_pixels, beam_y_pixels = detector_params.get_beam_position_pixels( | |
| detector_params.detector_distance | |
| ) | |
| yield from bps.abs_set(eiger.drv.beam_centre_x, beam_x_pixels) | |
| yield from bps.abs_set(eiger.drv.beam_centre_y, beam_y_pixels) | |
| yield from bps.abs_set(eiger.drv.det_distance, detector_params.detector_distance) | |
| yield from bps.abs_set(eiger.drv.omega_start, detector_params.omega_start) | |
| yield from bps.abs_set(eiger.drv.omega_increment, detector_params.omega_increment) | |
| def set_mx_settings_pvs(eiger: EigerDetector, detector_params: DetectorParams, group="mx_settings", wait=True): | |
| beam_x_pixels, beam_y_pixels = detector_params.get_beam_position_pixels( | |
| detector_params.detector_distance | |
| ) | |
| yield from bps.abs_set(eiger.drv.beam_centre_x, beam_x_pixels, group) | |
| yield from bps.abs_set(eiger.drv.beam_centre_y, beam_y_pixels, group) | |
| yield from bps.abs_set(eiger.drv.det_distance, detector_params.detector_distance, group) | |
| yield from bps.abs_set(eiger.drv.omega_start, detector_params.omega_start, group) | |
| yield from bps.abs_set(eiger.drv.omega_increment, detector_params.omega_increment, group) | |
| if wait: | |
| yield from bps.wait(group) |
This is a pattern we've used in a few places e.g. https://github.com/DiamondLightSource/mx-bluesky/blob/11c8a31f05dfc8421b9ebf205cc49598cd8806be/src/mx_bluesky/beamlines/i24/serial/setup_beamline/setup_beamline.py#L58. It allows the caller to have some flexibility in timings.
We should do the same for other plans in this file too
|
Currently, this plan does the following arming chain: The following are missing:
I am operating under the assumption
|
|
Following the last commit, the plan now also includes: However, this does not set
|
|
This is now being addressed in #1187. |
2 participants
Resolves #1053
This PR is not meant to be merged; kept as a draft to monitor changes to the dev branch.